Skip to content

fix(auth): match production's authenticate failure shape per grant - #53

Merged
gjtorikian merged 12 commits into
workos:mainfrom
grayashh:fix/authenticate-error-shapes
Aug 7, 2026
Merged

fix(auth): match production's authenticate failure shape per grant#53
gjtorikian merged 12 commits into
workos:mainfrom
grayashh:fix/authenticate-error-shapes

Conversation

@grayashh

@grayashh grayashh commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Production does not use one failure shape for POST /user_management/authenticate. Which grants fail OAuth-style is an explicit allowlist, not a category: password is an RFC 6749 grant and still fails with the plain {code, message} shape, verified against a live environment. The spec settles the rest (probe transcript in #51).

  • authorization_code (unknown, expired, or bad code_verifier) and refresh_token (unknown, expired, rotated, or user deleted) now render {error, error_description} via an OauthApiError subclass. This is the class the Node SDK's OauthException exists for, and what authkit-nextjs matches (error === "invalid_grant") to end a session when a refresh fails — against the emulator that path previously fell through to GenericServerException.
  • Device-code polling joins them: the spec renders expired_token, authorization_pending, slow_down and access_denied as {error, error_description}. These already carried OAuth error codes in the plain envelope, so a client matching error saw nothing and one matching code worked — the inverse of every other grant.
  • PKCE verifier mismatch also fails invalid_grant, the same way an unknown code does (RFC 7636 §4.6). This is the one case decided by reasoning rather than a probe: the spec does not enumerate invalid_grant for authenticate at all even though the live API returns it, so its silence is not evidence against.
  • password moves 401 → 400 and its message now interpolates the email (Invalid credentials for 'x@y.test'.), matching live. The shape stays plain. Anything asserting 401 on a bad password needs updating.
  • Magic auth code failures keep the plain shape but adopt the live code strings: invalid_one_time_code / "Invalid one-time code" and one_time_code_expired / "One-time code for '…' has expired." (were invalid_code / expired_code).
  • /sso/token is OAuth-shaped throughout, matching its spec definition — unsupported_grant_type for a wrong grant, invalid_grant for a bad or expired code, and invalid_request for a missing code (RFC 6749 §5.2), so the one failure a client hits before it has a code is not also the one it cannot parse like the rest.
  • /oauth2/token now throws the same OauthApiError instead of hand-building an identical body through a local oauthError() helper, leaving one definition of the OAuth envelope rather than two.
  • authentication.*_failed event payloads keep the spec's {code, message} error object: OauthApiError extends WorkOSApiError reusing the same fields, so failAuth and the error hooks need no change. Event error codes do change with the responses (invalid_codeinvalid_grant).
  • Left untouched for lack of live evidence: the email-verification grant's code strings and the standalone /user_management/email_verification route.

Closes #51

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown

Greptile Summary

The PR aligns authentication failures with production’s grant-specific response contracts.

  • Adds a shared OauthApiError renderer for {error, error_description} responses.
  • Applies OAuth-shaped failures to authorization-code, refresh-token, device-code, SSO, and OAuth token paths while retaining plain errors where production does.
  • Handles deleted users before authorization artifacts are consumed and adds coverage for the corrected failure paths.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/core/middleware/error-handler.ts Introduces the OAuth-specific error subclass and serializes it through the shared API error handler.
src/workos/routes/auth.ts Updates grant-specific error contracts and closes the deleted-user refresh-token and device-code fallthroughs.
src/workos/routes/oauth.ts Replaces the local OAuth response helper with the shared OAuth error class.
src/workos/routes/sso.ts Makes caller-caused token failures consistently OAuth-shaped with grant-appropriate codes.
src/workos/routes/auth.spec.ts Adds focused coverage for error envelopes, deleted users, expiration, PKCE, and device-code polling.
src/workos/routes/sso.spec.ts Covers OAuth-shaped SSO failures, event payloads, and missing or unsupported grant types.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Authentication request] --> B{Failure type}
    B -->|Malformed request| C[OauthApiError: invalid_request]
    B -->|Authorization code, refresh token, or device code failure| D[OauthApiError: grant-specific error]
    B -->|Password or Magic Auth credential failure| E[WorkOSApiError: code and message]
    B -->|Valid grant| F[Issue authentication response]
    C --> G[error and error_description envelope]
    D --> G
    E --> H[code and message envelope]
Loading

Reviews (8): Last reviewed commit: "docs(sso): name the profile-missing 500 ..." | Re-trigger Greptile

Comment thread src/workos/routes/auth.ts
grayashh and others added 4 commits August 6, 2026 19:39
The spec settles what probing could not. Its authenticate 400 lists the
device-flow codes (expired_token, authorization_pending, slow_down,
access_denied) as {error, error_description}, gives /sso/token nothing but
OAuth-shaped errors, and puts invalid_credentials in the plain set at 400.

That leaves the rule as an explicit allowlist rather than "RFC 6749 grants
fail OAuth-style": password is an RFC 6749 grant and still fails plain,
verified against a live environment. Stating it as a category was going to
mislead the next reader, since the category does not predict the shape.

PKCE is the one case decided by reasoning rather than evidence: the spec
does not enumerate invalid_grant for authenticate at all, even though the
live API returns it, so silence there is not evidence against. RFC 7636
§4.6 makes a failed verifier an invalid_grant, and it is the same grant on
the same endpoint already verified to fail that way.
Review follow-ups, all narrowing the gap between what the comments claim
and what the code does.

/sso/token's missing-code path still threw the plain envelope, directly
under a new comment asserting the endpoint answers OAuth-shaped throughout.
RFC 6749 §5.2 names a missing required parameter invalid_request, and it is
the one failure a client meets before it has a code to present — the worst
one to make it parse differently from the rest.

/oauth2/token had its own oauthError() building the identical
{error, error_description} body by hand, so the OAuth envelope was defined
in two places and the reusable one was reachable from everywhere except the
endpoint most obviously about OAuth. Throwing OauthApiError leaves one
definition; the m2m tests pass untouched, which is the point.

Three shapes shipped with no test: the expired refresh token's distinct
description, the device flow's expired_token (a polling client stops there
where authorization_pending tells it to keep going), and /sso/token's
unsupported_grant_type, which had no coverage before this change either.
@gjtorikian gjtorikian changed the title fix(auth): render RFC 6749 grant failures OAuth-style and adopt production magic-auth error codes fix(auth): match production's authenticate failure shape per grant Aug 6, 2026
Comment thread src/workos/routes/auth.ts
An approved device code whose user was deleted fell through to the shared
lookup and answered with a {message, code} 404 — the one envelope this
endpoint otherwise never returns, on the grant whose whole contract is that a
polling client reads `error` to decide whether to keep going. The
refresh_token twin of this was already fixed; this mirrors it, and throws
before the delete so nothing is spent on a failure polling cannot resolve.

/sso/token reported an omitted grant_type as "not supported: undefined",
which names neither the problem nor anything the caller sent. Absent is a
malformed request — RFC 6749 §5.2 invalid_request — not a request for a grant
the endpoint declines to support.

The device-flow comment also implied slow_down and access_denied are among
the codes returned here. The spec defines them; the emulator has no
polling-interval or user-denial surface to emit either from.

And documents both error classes where error hooks are described, since a
hook that raises a failure rather than describing one now has two envelopes
to choose between.
e09d42e replaced "RFC 6749 grants fail OAuth-style" with an explicit
allowlist precisely because the category did not predict the shape and
was going to mislead the next reader. The allowlist then grew a third
member — device_code — without the prose following it, so both the
README and the comment beside the password grant still said two, one
line above a table listing three.

A reader checking whether their grant is OAuth-shaped counts the rows,
not the sentence, but a sentence that disagrees with the table beneath
it costs them the trust that makes the table worth reading.
The two remaining shapes this branch changed without pinning. Both
expired-code paths moved off the plain envelope — authenticate's
authorization_code from expired_code, /sso/token's from
expired_code — and neither had a test, so the only evidence they
render invalid_grant was that their unknown-code siblings do.

They are not those siblings with a different label. /sso/token's
expired branch resolves the profile behind the code first, so the
authentication.sso_failed it emits carries the organization and
connection the unknown-code event leaves null, and it consumes the
authorization where the unknown one has nothing to consume. Asserting
the org and connection is also what proves the test reached the
expired branch at all rather than falling through to the unknown one.

Both mint a real code and back-date it, matching how the expired
refresh token and device code are already tested, so an expiry that
stops being detected fails here rather than passing as a bad code.
The third instance of a hole already closed twice on this branch. Deleting
a user cascades to its sessions, memberships, factors, identities, password
resets, email verifications and magic auths — but not to its authorization
codes, so a code outlives the user it was minted for.

Redeeming one fell through to the shared lookup and answered 404
{"message":"User not found","code":"not_found"}: the spec shapes an
authenticate 404 as a bare {message}, so there is no `error` for a client
matching invalid_grant and no `code` worth reading either. It is also the
grant an AuthKit callback actually takes, and the path authkit-nextjs uses
to decide a session is over.

Guarded before the delete, like the device code's twin of this, so a
failure no retry can fix does not also cost the caller their code. Routed
through failAuth rather than a bare throw, because every other
authorization_code failure emits authentication.oauth_failed and this one
was silently skipping it.
The spec settles what the last pass had to reason about. Its authenticate
400 lists `invalid_request` among the {error, error_description} variants
and nowhere among the {code, message} ones, so a missing or unrecognized
parameter is OAuth-shaped on every grant — including the grants whose
credential failures are plain. The envelope is a property of the failure,
not only of the grant, and the eleven throws here now say so.

This is the argument 1ab8ec2 made for /sso/token's missing-code path,
applied to the endpoint it was skipped on. It was sharpest on PKCE, where a
wrong code_verifier answered {error, error_description} and a missing one
answered {code, message} two lines away: the same grant, the same request,
two envelopes depending on which way the client got it wrong.

The unrecognized-grant_type branch keeps `invalid_request` rather than
moving to `unsupported_grant_type`, which appears exactly once in the whole
spec, under /sso/token, and never in authenticate's 400. The asymmetry
reads as real rather than an omission: authenticate's body is a oneOf
discriminated on grant_type, so an unknown value fails body validation
instead of reaching a handler that could decline it. What changes is the
message, which claimed "Unsupported grant_type" under a code that says
malformed request.

Also corrects the PKCE comment. e09d42e recorded that the spec does not
enumerate invalid_grant for authenticate at all, which is why that case
was decided by RFC 7636 alone; it does enumerate it, as an
{error, error_description} variant, so the case is spec-backed like its
siblings.
1ab8ec2 asserted that /sso/token answers OAuth-shaped throughout, and the
test beside it went further: "the endpoint has no plain-shaped response for
a caller to have to parse." Twenty lines below the comment, a stored
authorization pointing at a missing profile throws a plain 500.

The code is right — that is emulator state gone wrong, not a request anyone
can fix by sending something else, and RFC 6749 §5.2's code list covers
client errors only, so there is nothing to render it as. The claim is what
was wrong. Both now say every failure a caller can *cause*, and the branch
itself explains why it is the one that isn't.
@gjtorikian

Copy link
Copy Markdown
Collaborator

thanks!

@gjtorikian
gjtorikian merged commit 9028c0a into workos:main Aug 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants